-
Notifications
You must be signed in to change notification settings - Fork 125
Add CI action for uploading Docker image for wheels #3268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
8f6ba67
to
0e6433d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3268 +/- ##
=======================================
Coverage 67.08% 67.08%
=======================================
Files 571 571
Lines 111039 111039
=======================================
Hits 74485 74485
Misses 36554 36554 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
✔️ 0e6433d -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
✔️ 1904ec2 -> Azure artifacts URL |
✔️ dcc62f8 -> Azure artifacts URL |
✔️ 0e3c6a7 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
✔️ de6cb90 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
It turns out that the action is very slow to build the aarch64 image since it's using emulation instead of a native runner, so I for now it's easier and faster to just build things locally, and push to dockerhub/GHCR. Native Linux arm runners are on the way, but only in early 2025, so I'll leave this unmerged for now. |
Remove Python 3.13t (the free-threaded build) as we do not support it yet.
|
✔️ c5429d6 -> Azure artifacts URL |
An update on this: GitHub released ARM runners recently 🎉 https://github.blog/changelog/2025-01-16-linux-arm64-hosted-runners-now-available-for-free-in-public-repositories-public-preview/ Note that, with this recent update, we could in principle move all of our CI infrastructure to GitHub (if this is desired). |
✔️ 7d5e83a -> Azure artifacts URL |
|
||
container_registry: | ||
description: 'The name of the container registry to upload the image to (only useful if used with upload=true)' | ||
default: 'ghcr.io' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: need to specify auth token.
|
||
tag: | ||
description: 'The tag for the final Docker image' | ||
default: 'latest-' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In light of #3306, the new images have the tag manylinux_2_28
, so maybe this needs some rethinking.
|
✔️ 352dd48 -> Azure artifacts URL |
The way this would ideally work is as follows:
Unfortunately, all of this is rather complex to set-up (also has security implications due to DockerHub not allowing auth tokens for orgs, and scoped ones at that), and having the split between Azure and GitHub Actions is not helping, so I'm kind of leaving this in the "nice idea, but complex implementation" bucket. |
The images on https://hub.docker.com/r/neuronsimulator/neuron_wheel/tags are a bit inconsistent; we should use one tag for all platforms (since Docker picks the right one based on the architecture when pulling, no need to have separate tags). They are also a bit of a pain to update: someone (with push permissions!) needs to build the image on their own machine, for both x86_64 and aarch64, tag them correctly, and then upload them to DockerHub. This whole process is a bit error-prone and can take a while.
This CI introduces a manual action that can be run (ran?) to automatically build the
neuronsimulator/neuron_wheel
image for both x86 and aarch64, and optionally pushes it to DockerHub (by settingupload
totrue
).Note that
DOCKERHUB_USERNAME
needs to be set as an env variable for this repo, andDOCKERHUB_TOKEN
needs to be set as an env secret for pushing to work.Once this is merged, and this action is ran at least once, I can make another PR which introduces the necessary changes to use a unified
neuronsimulator/neuron_wheel:latest
image everywhere.EDIT: after spending some time fiddling with it,
docker buildx
is a bit cumbersome to use locally, so in the interest of simplicity, the tags should bex86_64
andaarch64
(to match the outputs ofuname -m
).